Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Waffle.Ecto.Type load and dump in embeds #44

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

timadevelop
Copy link

By default Ecto uses :self strategy which encodes the type without calling load/1 or dump/1 on custom type. So Waffle.Ecto.Type was encoded and decoded as a json, with native Ecto tools -> it was stored as an actual json in DB instead of using filename_with_timestamp string representation defined in Waffle.Ecto.Type

BREAKING CHANGE: this commit will "break" loading existing embedded fieds from DB that used previous waffle versions. Now Waffle.Ecto.Type expects value to be a string, but old representation will feed it a map. Depending on your ecto adapter, you can fix it by: a) re-dumping all values to your db properly
b) adding Waffle.Ecto.Type.load/2 for a json Map values and parsing "updated_at" map value to NaiveDateTime from a representation specific to your adapter


@achempion I'm not sure how you approach breaking changes in this project, but I believe handling old (and wrong) data representation inside the library would be messy, so I didn't. E.g. we would end up having code like

  defp properly_parse_old_representation_for_ecto_adapter(...) do
    # I guess this function will heavily depend on a specific ecto adapter client uses
    ...
  end

  def load(definition, %{file_name: file_name, updated_at: updated_at}) do
    load(definition, %{"file_name" => file_name, "updated_at" => updated_at})
  end

  def load(_definition, %{"file_name" => file_name, "updated_at" => updated_at}) do
    ndt = properly_parse_old_representation_for_ecto_adapter(updated_at)
    {:ok, %{file_name: file_name, updated_at: ndt}}
  end

Seems like it's better to tell lib users who need this to handle their specific situation properly.
I'm not an expert in elixir or ecto though, so if someone knows a better way to make this backward compatible - feel free to add more commits.

- fixes elixir-waffle#19

By default Ecto uses :self strategy which encodes the type without
calling load/1 or dump/1 on custom type. So Waffle.Ecto.Type was
encoded and decoded as a json, with native Ecto tools -> it was stored
as an actual json in DB instead of using `filename_with_timestamp`
string representation defined in Waffle.Ecto.Type

BREAKING CHANGE: this commit will "break" loading existing embedded
fieds from DB that used previous waffle versions. Now Waffle.Ecto.Type
expects `value` to be a string, but old representation will feed it
a map. Depending on your ecto adapter, you can fix it by:
a) re-dumping all values to your db properly
b) adding Waffle.Ecto.Type.load/2 for a json Map values and parsing
"updated_at" map value to NaiveDateTime from a representation specific
to your adapter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

field updated_at waffle.Ecto.type in a embed schema
1 participant